-
Notifications
You must be signed in to change notification settings - Fork 560
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Treatment Summary UI Enhancement #10563
base: develop
Are you sure you want to change the base?
Treatment Summary UI Enhancement #10563
Conversation
WalkthroughThe changes involve the removal of the Changes
Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/components/Patient/MedicationStatementList.tsx (1)
179-179
: 🛠️ Refactor suggestionRemove inconsistent border styling.
The Card component still has
border-none
which conflicts with the parent component's border styling in TreatmentSummary.- <Card className={cn("border-none rounded-sm", className)}> + <Card className={cn("rounded-sm", className)}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/Patient/MedicationStatementList.tsx
(2 hunks)src/components/Patient/TreatmentSummary.tsx
(3 hunks)src/components/Patient/allergy/list.tsx
(3 hunks)src/components/Patient/diagnosis/list.tsx
(3 hunks)src/components/Patient/symptoms/list.tsx
(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (5)
src/components/Patient/TreatmentSummary.tsx (1)
194-230
: LGTM! Consistent border styling across components.The border styling changes are consistent across all components (AllergyList, SymptomsList, DiagnosisList, MedicationStatementList), improving the visual consistency of the treatment summary.
src/components/Patient/diagnosis/list.tsx (1)
142-142
: LGTM! Consistent Card styling.The Card styling changes align with other components, maintaining visual consistency.
src/components/Patient/symptoms/list.tsx (1)
140-140
: LGTM! Consistent Card styling.The Card styling changes align with other components, maintaining visual consistency.
src/components/Patient/allergy/list.tsx (2)
98-98
: LGTM! Consistent className prop forwarding.The
className
prop is now consistently forwarded toAllergyListLayout
across all rendering paths (loading, empty, and main states), improving the component's styling flexibility.Also applies to: 122-122
310-310
: LGTM! Enhanced Card styling with proper className composition.The styling changes:
- Add consistent padding with
p-2
- Apply subtle border radius with
rounded-sm
- Maintain className extensibility through proper composition with the
cn()
utility
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Don't need colors for the badges here: ![]() Modify the filteredMedications length check to display no medications recorded, and the table row styling as well when there's data: ![]() cc: @amjithtitus09 |
Is this done @AdityaJ2305 ? |
@amjithtitus09 ![]() LMK your thoughts
|
@amjithtitus09 Done with the changes . Ready for review 🙂 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/Patient/diagnosis/DiagnosisTable.tsx (1)
74-104
: Consider extracting the badge styling logic.The badge styling logic is duplicated across multiple badges. Consider extracting it into a reusable helper function or component.
+const getBadgeClassName = (isPrintPreview: boolean, styles: string) => + isPrintPreview ? "" : `whitespace-nowrap ${styles}`; <Badge variant="outline" - className={ - isPrintPreview - ? "" - : `whitespace-nowrap ${ - DIAGNOSIS_CLINICAL_STATUS_STYLES[ - diagnosis.clinical_status - ] - }` - } + className={getBadgeClassName( + isPrintPreview, + DIAGNOSIS_CLINICAL_STATUS_STYLES[diagnosis.clinical_status] + )} >src/components/Patient/symptoms/SymptomTable.tsx (1)
75-87
: Consider sharing badge styling logic between components.The badge styling implementation is identical to DiagnosisTable.tsx. Consider:
- Creating a shared helper function for badge styling
- Creating a shared styled badge component
This would improve maintainability and ensure consistent behavior across components.
// src/components/Common/PrintAwareBadge.tsx interface PrintAwareBadgeProps { isPrintPreview?: boolean; styles: string; children: React.ReactNode; } export function PrintAwareBadge({ isPrintPreview = false, styles, children, ...props }: PrintAwareBadgeProps) { return ( <Badge variant="outline" className={isPrintPreview ? "" : `whitespace-nowrap ${styles}`} {...props} > {children} </Badge> ); }Usage:
-<Badge - variant="outline" - className={ - isPrintPreview - ? "" - : `whitespace-nowrap ${ - SYMPTOM_SEVERITY_STYLES[symptom.severity] - }` - } -> +<PrintAwareBadge + isPrintPreview={isPrintPreview} + styles={SYMPTOM_SEVERITY_STYLES[symptom.severity]} +> {t(symptom.severity)} -</Badge> +</PrintAwareBadge>Also applies to: 89-101, 103-117
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/components/Patient/MedicationStatementList.tsx
(2 hunks)src/components/Patient/allergy/list.tsx
(4 hunks)src/components/Patient/diagnosis/DiagnosisTable.tsx
(1 hunks)src/components/Patient/diagnosis/list.tsx
(3 hunks)src/components/Patient/symptoms/SymptomTable.tsx
(1 hunks)src/components/Patient/symptoms/list.tsx
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/components/Patient/diagnosis/list.tsx
- src/components/Patient/MedicationStatementList.tsx
- src/components/Patient/symptoms/list.tsx
- src/components/Patient/allergy/list.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (3)
src/components/Patient/diagnosis/DiagnosisTable.tsx (2)
74-88
: LGTM! Clinical status badge styling implementation.The conditional styling based on
isPrintPreview
aligns with the PR objectives and reviewer feedback about badge colors.
90-104
: LGTM! Verification status badge styling implementation.The conditional styling based on
isPrintPreview
aligns with the PR objectives and reviewer feedback about badge colors.src/components/Patient/symptoms/SymptomTable.tsx (1)
75-117
: Verify badge styling behavior in print preview.The changes align with the PR objectives and reviewer feedback. However, let's verify the print preview appearance matches the expected design.
✅ Verification successful
🌐 Web query:
What is the recommended way to test print preview in React applications?
💡 Result:
To test print preview functionality in React applications effectively, developers should combine specific testing strategies and leverage the react-to-print library's features. Here's a structured approach:
1. Component Inspection with React Testing Library
- Use screen.debug() to inspect rendered components:
test('verify printable content', () => { render(<PrintableComponent />); screen.debug(undefined, Infinity); // Shows full component structure[10] });- This exposes hidden elements wrapped in
<div style={{ display: "none" }}>
that are only visible during printing[3][6].2. State Synchronization Testing
- Test asynchronous state handling required for printing:
test('state updates before printing', async () => { const { getByText } = render(<PrintButton />); fireEvent.click(getByText('Print')); await waitFor(() => expect(screen.getByTestId('print-content')).toBeVisible()); });- Ensures components properly use Promises to wait for DOM updates[2][3][7].
3. Print Iframe Preservation
- Enable preserveAfterPrint flag during testing:
<ReactToPrint preserveAfterPrint={true} content={() => componentRef.current} />- Allows post-print inspection of generated iframe content[7].
4. CSS Media Query Verification
Test print-specific styles using:
@media print { .no-print { display: none; } }Validate styles with tools like:
expect(getComputedStyle(element).display).toBe('none');5. Browser Print Dialog Mocking
Mock window.print() in tests:
const printMock = jest.spyOn(window, 'print').mockImplementation(); // ... test execution ... expect(printMock).toHaveBeenCalled();Key Considerations
Aspect Implementation Source Hidden Components Use display: none
wrapper[3][7] Async State Promise resolution in onBeforePrint
[2][3][7] Style Isolation Dedicated @media print CSS [1][4] Debugging preserveAfterPrint + screen.debug [7][10] For comprehensive testing, combine visual inspection of preserved print iframes with state/behavior assertions to ensure all printable elements render correctly while non-essential UI remains hidden[1][3][7].
Citations:
- 1: https://blog.bytescrum.com/mastering-react-print-a-comprehensive-guide
- 2: https://buaiscia.github.io/blog/tips/printing-developer-journey-react-to-print
- 3: https://github.com/MatthewHerbst/react-to-print
- 4: https://www.dhiwise.com/post/boosting-efficiency-optimizing-your-react-to-print-workflow
- 5: https://dev.to/ebereplenty/printing-in-react-made-easy-with-react-to-print-4b3k
- 6: https://www.browserstack.com/guide/react-testing-library-debug-method
- 7: https://www.npmjs.com/package/react-to-print
- 8: https://blog.logrocket.com/using-react-to-print-generate-printable-document/
- 9: https://supportcenter.devexpress.com/ticket/details/t1052262/react-print-report-without-preview-doesn-t-work
- 10: https://javascript.plainenglish.io/how-to-print-the-entire-component-using-screen-debug-in-react-testing-library-330db38d3e78?gi=cf5c55e36588
Print Preview Badge Styling: Behavior Verified
- The conditional logic using
isPrintPreview
correctly bypasses the addition of extra style classes in print preview mode.- The code omits badge-specific styling when printing, which is in line with the expected design.
- Recommended testing approaches (e.g., with React Testing Library and react-to-print) support verifying that print-specific rendering omits non-print styles.
In the right direction but few more changes needed here. There is still bg colour, and coloured labels. Also border needs to be consistent for all tables. Check if there's shadow for the questionnaire responses at the bottom. |
@amjithtitus09n, Let me know ur input I just want to confirm if I’ve understood the requirements correctly: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/Patient/MedicationStatementList.tsx
(7 hunks)src/components/Patient/diagnosis/DiagnosisTable.tsx
(3 hunks)src/components/Patient/diagnosis/list.tsx
(6 hunks)src/components/Patient/symptoms/SymptomTable.tsx
(3 hunks)src/components/Patient/symptoms/list.tsx
(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/components/Patient/diagnosis/list.tsx
- src/components/Patient/symptoms/SymptomTable.tsx
- src/components/Patient/diagnosis/DiagnosisTable.tsx
- src/components/Patient/MedicationStatementList.tsx
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Redirect rules - care-ohc
- GitHub Check: Header rules - care-ohc
- GitHub Check: Pages changed - care-ohc
- GitHub Check: Test
- GitHub Check: cypress-run (1)
🔇 Additional comments (3)
src/components/Patient/symptoms/list.tsx (3)
14-17
: LGTM! Clean import additions.The new imports for
PrintTable
andformatName
are properly organized and align with the component's enhanced functionality.
96-115
: LGTM! Clean print preview implementation.The
PrintTable
implementation provides a well-structured print-friendly view of symptoms data with appropriate column headers and data formatting.
168-174
:❓ Verification inconclusive
Verify complete removal of shadows in print preview mode.
The current implementation removes shadows using
shadow-none
, but per amjithtitus09's feedback, please verify that all shadows are removed, including those in questionnaire responses.Run this script to check for any remaining shadow styles:
🏁 Script executed:
#!/bin/bash # Search for shadow-related styles in the component and its dependencies rg -i '(shadow|elevation)' src/components/Patient/symptoms/Length of output: 155
Print Preview Shadow Removal Verification
The search output confirms that the only shadow-related style in the
src/components/Patient/symptoms/list.tsx
component is"shadow-none border-none"
, which correctly removes shadows in print preview mode. However, per amjithtitus09’s feedback, please verify that any components handling questionnaire responses also have all shadows removed in print preview mode.
- Confirm that all relevant questionnaire response components apply similar shadow removal styles.
- Ensure no residual shadow or elevation classes are present in those components.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/components/Patient/MedicationStatementList.tsx (1)
62-69
:⚠️ Potential issueRemove badge color styles as requested.
Based on the PR feedback from Jacobjeevan and amjithtitus09, the badge colors should be removed.
Apply this diff to remove the color styles:
- <Badge - variant="outline" - className={`whitespace-nowrap capitalize ${ - MEDICATION_STATEMENT_STATUS_STYLES[statement.status] - }`} - > + <Badge variant="outline" className="whitespace-nowrap capitalize"> {statement.status} </Badge>
🧹 Nitpick comments (2)
src/components/Patient/MedicationStatementList.tsx (2)
246-260
: Consider accessibility improvements for the "View All" button.While the implementation is functional, the button's styling might have accessibility issues:
- Using both
underline
and low contrasttext-gray-500
might affect readability- The
xs
size might make it difficult to click on mobile devicesConsider this improvement:
- <Button - variant="ghost" - size="xs" - onClick={() => setShowEnteredInError(true)} - className="text-xs underline text-gray-500" - > + <Button + variant="link" + size="sm" + onClick={() => setShowEnteredInError(true)} + className="text-sm text-primary hover:text-primary/80" + > {t("view_all")} </Button>
293-296
: Remove unnecessary whitespace in title.There's an unnecessary whitespace character that should be removed.
Apply this diff:
<CardTitle> - {" "} {t("ongoing_medications")} ({medicationsCount && medicationsCount}) </CardTitle>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Patient/MedicationStatementList.tsx
(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (4)
src/components/Patient/MedicationStatementList.tsx (4)
2-2
: LGTM! Clean refactoring of imports and props.The removal of
isPrintPreview
fromMedicationRowProps
and the addition of necessary imports align well with the architectural changes to handle print preview at a higher level.Also applies to: 27-27, 43-46
78-96
: LGTM! Consistent note display implementation.The Popover implementation for notes provides a consistent user experience across all states.
169-198
: LGTM! Clean print view implementation.The
PrintTable
implementation provides a clean, print-friendly view of the medication data.
267-308
: LGTM! Clean layout component implementation.The new
MedicationStatementListLayout
component effectively encapsulates the layout logic and print-specific styling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/components/Medicine/MedicationsTable.tsx (1)
118-157
: 🛠️ Refactor suggestionUse shared data transformation function for consistency.
After extracting the shared data transformation logic, update the regular table implementation to use it as well.
- const instruction = medication.dosage_instruction[0]; - const frequency = getFrequencyDisplay(instruction?.timing); - const dosage = formatDosage(instruction); - const duration = instruction?.timing?.repeat?.bounds_duration; - const remarks = formatSig(instruction); - const notes = medication.note; + const { + medicine, + dosage, + frequency, + duration, + instructions, + } = transformMedicationData(medication); const isInactive = INACTIVE_MEDICATION_STATUSES.includes( medication.status as (typeof INACTIVE_MEDICATION_STATUSES)[number], ); return ( <TableRow key={medication.id} className={`divide-x font-medium ${isInactive ? "bg-gray-100" : ""}`} > - <TableCell className="py-2 px-3"> - {medication.medication?.display} - </TableCell> - <TableCell className="py-2 px-3">{dosage}</TableCell> - <TableCell className="py-2 px-3"> - {instruction?.as_needed_boolean - ? `${t("as_needed_prn")} (${instruction?.as_needed_for?.display})` - : frequency?.meaning} - {instruction?.additional_instruction?.[0]?.display && ( - <div className="text-sm text-gray-600"> - {instruction.additional_instruction[0].display} - </div> - )} - </TableCell> - <TableCell className="py-2 px-3"> - {duration ? `${duration.value} ${duration.unit}` : "-"} - </TableCell> - <TableCell className="py-2 px-3"> - {remarks || "-"} - {notes ? ` (${t("note")}: ${notes})` : ""} - </TableCell> + <TableCell className="py-2 px-3">{medicine}</TableCell> + <TableCell className="py-2 px-3">{dosage}</TableCell> + <TableCell className="py-2 px-3">{frequency}</TableCell> + <TableCell className="py-2 px-3">{duration}</TableCell> + <TableCell className="py-2 px-3">{instructions}</TableCell> </TableRow> );
🧹 Nitpick comments (1)
src/components/Medicine/MedicationsTable.tsx (1)
67-73
: Enhance empty state styling for better consistency.The empty state implementation aligns with the PR feedback. However, consider matching the styling with the table's border and rounded corners for visual consistency.
- <CardContent className="p-2"> + <CardContent className="border rounded-lg p-4"> <p className="text-gray-500">{t("no_medication_recorded")}</p> </CardContent>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Medicine/MedicationsTable.tsx
(4 hunks)src/components/Patient/TreatmentSummary.tsx
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Patient/TreatmentSummary.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (2)
src/components/Medicine/MedicationsTable.tsx (2)
4-4
: LGTM! Interface and imports updated correctly.The changes appropriately introduce the print preview functionality by adding the necessary imports and updating the component interface.
Also applies to: 15-15, 40-44, 46-50
75-105
: LGTM! Print preview implementation meets requirements.The print preview implementation correctly handles all necessary data fields and aligns with the PR objectives to enhance the Treatment Summary UI.
@amjithtitus09 , Ready for review and testing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (4)
src/components/Patient/diagnosis/list.tsx (1)
90-103
: Optimize filtering logic to avoid duplicate array operations.The current implementation filters the array twice. Consider simplifying the logic to improve performance.
- diagnoses={[ - ...filteredDiagnoses.filter( - (diagnosis) => - diagnosis.verification_status !== "entered_in_error", - ), - ...(showEnteredInError - ? filteredDiagnoses.filter( - (diagnosis) => - diagnosis.verification_status === "entered_in_error", - ) - : []), - ]} + diagnoses={filteredDiagnoses.filter((diagnosis) => + showEnteredInError || diagnosis.verification_status !== "entered_in_error" + )}src/components/Patient/TreatmentSummary.tsx (3)
32-47
: Consider enhancing the SectionLayout component.The component could benefit from additional props for customization and accessibility.
const SectionLayout = ({ children, title, + className, + id, }: { title: string; children: React.ReactNode; + className?: string; + id?: string; }) => { return ( - <Card className="rounded-sm shadow-none border-none"> + <Card + className={`rounded-sm shadow-none border-none ${className ?? ''}`} + id={id} + > <CardHeader className="flex justify-between flex-row px-0 py-2 "> - <CardTitle>{title}</CardTitle> + <CardTitle role="heading" aria-level={2}>{title}</CardTitle> </CardHeader> <CardContent className="px-0 py-0">{children}</CardContent> </Card> ); };
441-448
: Consider moving QuestionnaireResponsesList to SectionLayout.For consistency with other sections, wrap the QuestionnaireResponsesList in a SectionLayout component.
- <div> + <SectionLayout title={t("questionnaire_responses")}> <QuestionnaireResponsesList encounter={encounter} patientId={encounter.patient.id} isPrintPreview={true} onlyUnstructured={true} /> - </div> + </SectionLayout>
452-456
: Enhance footer accessibility.The footer text is quite small (10px) which might affect readability. Consider using a slightly larger font size and adding ARIA attributes.
- <div className="mt-8 space-y-1 pt-2 text-[10px] text-gray-500 flex justify-between"> + <footer className="mt-8 space-y-1 pt-2 text-xs text-gray-500 flex justify-between" role="contentinfo"> <p> {t("generated_on")} {format(new Date(), "PPP 'at' p")} </p> - </div> + </footer>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/Routers/routes/ConsultationRoutes.tsx
(1 hunks)src/components/Facility/ConsultationDetails/QuestionnaireResponsesList.tsx
(2 hunks)src/components/Medicine/MedicationsTable.tsx
(2 hunks)src/components/Patient/MedicationStatementList.tsx
(7 hunks)src/components/Patient/TreatmentSummary.tsx
(8 hunks)src/components/Patient/allergy/list.tsx
(5 hunks)src/components/Patient/diagnosis/list.tsx
(6 hunks)src/components/Patient/symptoms/list.tsx
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/components/Medicine/MedicationsTable.tsx
- src/components/Facility/ConsultationDetails/QuestionnaireResponsesList.tsx
- src/components/Patient/MedicationStatementList.tsx
- src/components/Patient/symptoms/list.tsx
- src/components/Patient/allergy/list.tsx
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: AdityaJ2305
PR: ohcnetwork/care_fe#10563
File: src/components/Medicine/MedicationsTable.tsx:75-105
Timestamp: 2025-02-21T08:09:05.237Z
Learning: In the MedicationsTable component, the data transformation logic for print preview and regular table views is intentionally kept separate as they will handle arrays differently in future implementations.
src/components/Patient/TreatmentSummary.tsx (1)
Learnt from: AdityaJ2305
PR: ohcnetwork/care_fe#10563
File: src/components/Medicine/MedicationsTable.tsx:75-105
Timestamp: 2025-02-21T08:09:05.237Z
Learning: In the MedicationsTable component, the data transformation logic for print preview and regular table views is intentionally kept separate as they will handle arrays differently in future implementations.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (7)
src/components/Patient/diagnosis/list.tsx (3)
19-24
: LGTM! Clean interface definition.The
DiagnosisListProps
interface is well-defined with essential props and proper TypeScript types.
32-32
: LGTM! Simplified state management.Clean implementation of the error visibility state with a sensible default value.
139-142
: LGTM! Consistent layout styling.The layout component has been simplified with consistent spacing and proper use of the
cn
utility for class name management.Also applies to: 154-154
src/Routers/routes/ConsultationRoutes.tsx (2)
19-24
: LGTM! The route handler update is consistent and well-structured.The addition of
patientId
to the treatment summary route handler follows the established pattern in the codebase and ensures proper data flow to theTreatmentSummary
component.
18-25
:✅ Verification successful
Verify route path consistency across the codebase.
Let's ensure the route path format and parameter usage is consistent across the application.
🏁 Script executed:
#!/bin/bash # Description: Check route path patterns and parameter usage across the codebase. # Search for route paths with facilityId, patientId, and encounterId parameters echo "Checking route path patterns:" rg -A 1 '"/facility/:[^/]+/patient/:[^/]+/encounter/:[^/]+"' # Search for route handler parameter destructuring patterns echo "\nChecking parameter destructuring patterns:" ast-grep --pattern $'({ facilityId, encounterId, patientId })'Length of output: 294
Route Handler Consistency Verified
The verification confirms that the route path in
src/Routers/routes/ConsultationRoutes.tsx
correctly includes the:patientId
parameter and that the handler destructuring—extractingfacilityId
,encounterId
, andpatientId
—is consistent with the patterns used elsewhere in the codebase. No changes are needed.src/components/Patient/TreatmentSummary.tsx (2)
25-30
: LGTM! Props interface is well-defined.The addition of the
patientId
prop enhances the component's data fetching capabilities and aligns with the PR objectives.
69-113
: LGTM! Data fetching logic is well-structured.The implementation of data fetching using React Query is clean and follows best practices. The queries are properly keyed and include necessary dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/Patient/TreatmentSummary.tsx (2)
69-113
: Add error handling for API calls.While the data fetching logic is well-organized, consider adding error handling for failed API calls to improve user experience.
const { data: allergies } = useQuery({ + onError: (error) => { + // Handle error appropriately + console.error('Failed to fetch allergies:', error); + }, queryKey: ["allergies", patientId, encounterId], queryFn: query(allergyIntoleranceApi.getAllergy, {
148-273
: Consider improving grid layout responsiveness.The grid layout for patient details uses fixed column widths (
grid-cols-[10rem,auto,1fr]
) which might not be optimal for very small screens. Consider using more flexible units or adding additional breakpoints.- <div className="grid grid-cols-[10rem,auto,1fr] md:grid-cols-[8rem,auto,1fr] items-center"> + <div className="grid grid-cols-[minmax(8rem,auto),auto,1fr] items-center">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Patient/TreatmentSummary.tsx
(8 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: AdityaJ2305
PR: ohcnetwork/care_fe#10563
File: src/components/Medicine/MedicationsTable.tsx:75-105
Timestamp: 2025-02-21T08:09:05.237Z
Learning: In the MedicationsTable component, the data transformation logic for print preview and regular table views is intentionally kept separate as they will handle arrays differently in future implementations.
src/components/Patient/TreatmentSummary.tsx (1)
Learnt from: AdityaJ2305
PR: ohcnetwork/care_fe#10563
File: src/components/Medicine/MedicationsTable.tsx:75-105
Timestamp: 2025-02-21T08:09:05.237Z
Learning: In the MedicationsTable component, the data transformation logic for print preview and regular table views is intentionally kept separate as they will handle arrays differently in future implementations.
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (4)
src/components/Patient/TreatmentSummary.tsx (4)
32-47
: LGTM! Well-structured reusable layout component.The
SectionLayout
component is well-designed, using shadcn/ui Card components for consistent styling and providing a clean interface for section headers and content.
49-55
: LGTM! Consistent empty state handling.The
EmptyState
component provides a clean and consistent way to handle empty states across different sections, with proper i18n support.
372-395
: LGTM! Well-structured medication data transformation.The medication data transformation logic is well-organized and handles all edge cases properly, including optional fields and fallback values.
126-126
: Fix typo in className.There's a typo in the className that breaks the min-height styling.
- <div className="min-h-scree py-2 max-w-4xl mx-auto"> + <div className="min-h-screen py-2 max-w-4xl mx-auto">
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/components/Patient/TreatmentSummary.tsx (3)
69-113
: Consider implementing error boundaries for API calls.Multiple API calls are made using
useQuery
. While the implementation is correct, consider adding error handling to gracefully handle API failures.const { data: allergies } = useQuery({ queryKey: ["allergies", patientId, encounterId], queryFn: query(allergyIntoleranceApi.getAllergy, { pathParams: { patientId }, queryParams: { encounter: ( encounter?.status ? completedEncounterStatus.includes(encounter.status) : false ) ? encounterId : undefined, }, + onError: (error) => { + // Handle error appropriately + console.error('Failed to fetch allergies:', error); + } }), });
372-395
: Consider extracting medication mapping logic.The medication data transformation logic is complex and could be moved to a separate utility function for better maintainability.
+const mapMedicationToTableRow = (medication) => { + const instruction = medication.dosage_instruction[0]; + const frequency = getFrequencyDisplay(instruction?.timing); + const dosage = formatDosage(instruction); + const duration = instruction?.timing?.repeat?.bounds_duration; + const remarks = formatSig(instruction); + const notes = medication.note; + + return { + medicine: medication.medication?.display, + status: t(medication.status), + dosage: dosage, + frequency: instruction?.as_needed_boolean + ? `${t("as_needed_prn")} (${instruction?.as_needed_for?.display ?? "-"})` + : (frequency?.meaning ?? "-") + + (instruction?.additional_instruction?.[0]?.display + ? `, ${instruction.additional_instruction[0].display}` + : ""), + duration: duration + ? `${duration.value} ${duration.unit}` + : "-", + instructions: `${remarks || "-"}${notes ? ` (${t("note")}: ${notes})` : ""}`, + }; +}; <PrintTable headers={[ { key: "medicine", title: t("medicine") }, { key: "status", title: t("status") }, { key: "dosage", title: t("dosage") }, { key: "frequency", title: t("frequency") }, { key: "duration", title: t("duration") }, { key: "instructions", title: t("instructions") }, ]} - rows={medications?.results.map((medication) => { - const instruction = medication.dosage_instruction[0]; - const frequency = getFrequencyDisplay(instruction?.timing); - const dosage = formatDosage(instruction); - const duration = instruction?.timing?.repeat?.bounds_duration; - const remarks = formatSig(instruction); - const notes = medication.note; - return { - medicine: medication.medication?.display, - status: t(medication.status), - dosage: dosage, - frequency: instruction?.as_needed_boolean - ? `${t("as_needed_prn")} (${instruction?.as_needed_for?.display ?? "-"})` - : (frequency?.meaning ?? "-") + - (instruction?.additional_instruction?.[0]?.display - ? `, ${instruction.additional_instruction[0].display}` - : ""), - duration: duration - ? `${duration.value} ${duration.unit}` - : "-", - instructions: `${remarks || "-"}${notes ? ` (${t("note")}: ${notes})` : ""}`, - }; - })} + rows={medications?.results.map(mapMedicationToTableRow)} />
442-449
: Consider adding loading state for QuestionnaireResponsesList.The
QuestionnaireResponsesList
component might benefit from a loading state while data is being fetched.<QuestionnaireResponsesList encounter={encounter} patientId={encounter.patient.id} isPrintPreview={true} onlyUnstructured={true} + isLoading={isLoading} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Facility/ConsultationDetails/QuestionnaireResponsesList.tsx
(3 hunks)src/components/Patient/TreatmentSummary.tsx
(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Facility/ConsultationDetails/QuestionnaireResponsesList.tsx
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: AdityaJ2305
PR: ohcnetwork/care_fe#10563
File: src/components/Medicine/MedicationsTable.tsx:75-105
Timestamp: 2025-02-21T08:09:05.237Z
Learning: In the MedicationsTable component, the data transformation logic for print preview and regular table views is intentionally kept separate as they will handle arrays differently in future implementations.
src/components/Patient/TreatmentSummary.tsx (1)
Learnt from: AdityaJ2305
PR: ohcnetwork/care_fe#10563
File: src/components/Medicine/MedicationsTable.tsx:75-105
Timestamp: 2025-02-21T08:09:05.237Z
Learning: In the MedicationsTable component, the data transformation logic for print preview and regular table views is intentionally kept separate as they will handle arrays differently in future implementations.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (4)
src/components/Patient/TreatmentSummary.tsx (4)
25-30
: LGTM! Props interface is well-defined.The
TreatmentSummaryProps
interface is clear and includes all necessary props for the component's functionality.
32-55
: LGTM! Well-structured reusable components.The
SectionLayout
andEmptyState
components are well-designed, promoting consistency and reusability across sections.
126-126
: Fix typo in className.There's a typo in the className that breaks the min-height styling.
277-300
: LGTM! Consistent table implementation across sections.The implementation of tables using
PrintTable
component is consistent across all sections (allergies, symptoms, diagnoses, medications, ongoing medications). This promotes maintainability and provides a uniform user experience.The empty state handling is also consistent, using the
EmptyState
component when no data is available.Also applies to: 304-327, 330-358, 361-400, 403-438
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/components/Patient/TreatmentSummary.tsx (4)
33-48
: Consider adding type safety to the SectionLayout component.The component could benefit from using React.FC type and children prop type definition.
-const SectionLayout = ({ +const SectionLayout: React.FC<{ + title: string; + children: React.ReactNode; +}> = ({ children, title, -}: { - title: string; - children: React.ReactNode; }) => {
70-115
: Consider optimizing query fetching with useQueries hook.Multiple independent queries could be optimized using the
useQueries
hook from React Query for better performance and cleaner code.- const { data: allergies, isLoading: allergiesLoading } = useQuery({...}); - const { data: symptoms, isLoading: symptomsLoading } = useQuery({...}); - const { data: diagnoses, isLoading: diagnosesLoading } = useQuery({...}); - const { data: medications, isLoading: medicationsLoading } = useQuery({...}); - const { data: medicationStatement, isLoading: medicationStatementLoading } = useQuery({...}); + const queries = useQueries({ + queries: [ + { + queryKey: ["allergies", patientId, encounterId], + queryFn: query(allergyIntoleranceApi.getAllergy, {...}), + }, + // ... other queries + ], + }); + const isLoading = queries.some(query => query.isLoading);
394-417
: Simplify medication data transformation logic.The medication data transformation logic is complex and could be simplified by extracting the transformation logic into a separate function.
+const transformMedicationData = (medication: any) => { + const instruction = medication.dosage_instruction[0]; + const frequency = getFrequencyDisplay(instruction?.timing); + return { + medicine: medication.medication?.display, + status: t(medication.status), + dosage: formatDosage(instruction), + frequency: instruction?.as_needed_boolean + ? `${t("as_needed_prn")} (${instruction?.as_needed_for?.display ?? "-"})` + : (frequency?.meaning ?? "-") + + (instruction?.additional_instruction?.[0]?.display + ? `, ${instruction.additional_instruction[0].display}` + : ""), + duration: instruction?.timing?.repeat?.bounds_duration + ? `${instruction.timing.repeat.bounds_duration.value} ${instruction.timing.repeat.bounds_duration.unit}` + : "-", + instructions: `${formatSig(instruction) || "-"}${medication.note ? ` (${t("note")}: ${medication.note})` : ""}`, + }; +}; rows={medications?.results.map(transformMedicationData)}
117-123
: Enhance error handling for edge cases.The error handling only checks for missing encounter. Consider handling other potential error cases and providing more informative error messages.
+ if (encounter?.error) { + return ( + <div className="flex h-[200px] items-center justify-center rounded-lg border-2 border-dashed p-4 text-gray-500"> + {t("error_loading_encounter")} + </div> + ); + } + if (!encounter) { return ( <div className="flex h-[200px] items-center justify-center rounded-lg border-2 border-dashed p-4 text-gray-500">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Patient/TreatmentSummary.tsx
(8 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: AdityaJ2305
PR: ohcnetwork/care_fe#10563
File: src/components/Medicine/MedicationsTable.tsx:75-105
Timestamp: 2025-02-21T08:09:05.237Z
Learning: In the MedicationsTable component, the data transformation logic for print preview and regular table views is intentionally kept separate as they will handle arrays differently in future implementations.
src/components/Patient/TreatmentSummary.tsx (1)
Learnt from: AdityaJ2305
PR: ohcnetwork/care_fe#10563
File: src/components/Medicine/MedicationsTable.tsx:75-105
Timestamp: 2025-02-21T08:09:05.237Z
Learning: In the MedicationsTable component, the data transformation logic for print preview and regular table views is intentionally kept separate as they will handle arrays differently in future implementations.
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Test
- GitHub Check: CodeQL-Build
- GitHub Check: OSSAR-Scan
- GitHub Check: lint
- GitHub Check: cypress-run (1)
🔇 Additional comments (1)
src/components/Patient/TreatmentSummary.tsx (1)
148-148
: Fix typo in className.There's a typo in the className that breaks the min-height styling.
- <div className="min-h-scree py-2 max-w-4xl mx-auto"> + <div className="min-h-screen py-2 max-w-4xl mx-auto">
Proposed Changes
PrintTable
comp to creat table for print purpose specificMedicationstatementLayout
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Summary by CodeRabbit
New Features
PrintTable
component for improved print-friendly displays.MedicationStatementList
,AllergyList
,DiagnosisList
, andSymptomsList
components to consistently render notes and data without relying on print preview mode.MedicationsTable
to show a message when no medication records are present.TreatmentSummary
component to a modular layout for better organization and presentation.patientId
to theTreatmentSummary
component for enhanced data handling.Style